Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(protocol): simplify some protocol code based on OpenZeppelin's recommendation #18308

Merged
merged 7 commits into from
Oct 26, 2024

Conversation

dantaik
Copy link
Contributor

@dantaik dantaik commented Oct 25, 2024

Throughout the codebase, multiple opportunities for code simplification were identified:

  • In the _proposeBlock function, the local.params object has a zero anchorBlockId and timestamp for pre-fork blocks. This means that the first part of the condition, the postFork flag, is redundant when setting default values.
  • This unchecked block does not wrap any math operations and can be removed.
  • Since local.params.timestamp is set to the L1 block timestamp before the fork, it covers both conditions when setting the block's proposedAt field.
  • This conditional statement is unnecessary because the computation returns zero when _amount is zero.
  • This else block is redundant because the if case breaks out of the loop.
    To improve code maintainability and clarity, consider implementing the aforementioned simplification suggestions.

@dantaik dantaik marked this pull request as ready for review October 25, 2024 09:04
@dantaik dantaik marked this pull request as draft October 25, 2024 09:04
Copy link

openzeppelin-code bot commented Oct 25, 2024

refactor(protocol): code simplifications

Generated at commit: e50b159b988e8ab1c6e3f95f7ace44a6d710b57b

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
3
3
0
7
43
56
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

@dantaik dantaik marked this pull request as ready for review October 25, 2024 09:34
@dantaik dantaik enabled auto-merge October 25, 2024 10:34
@dantaik dantaik changed the title refactor(protocol): code simplifications refactor(protocol): simplify some protocol code based on OpenZeppelin's recommendation Oct 25, 2024
@dantaik dantaik requested a review from YoGhurt111 October 25, 2024 14:51
@dantaik dantaik added this pull request to the merge queue Oct 26, 2024
Merged via the queue into main with commit fbad703 Oct 26, 2024
10 checks passed
@dantaik dantaik deleted the fix-N-09-Code-Simplifications branch October 26, 2024 05:00
dantaik added a commit that referenced this pull request Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants